I am against the suggestion in this change request because it contains arguably misleading caller instructions (where no instructions are necessary at all) and because the suggested wording implies that a caller has access to multiple option values. I do agree that "accessor" is also problematic and posted a change request with a specific replacement suggestion.

There is no change from the implications created by the member naming ( "AorB" ) except to clarify that hidden nullptr semantic meaning. I am open to alternatives that clarify the three semantic meanings better.

@yadij

There is no change from the implications created by the member naming ( "AorB" ) except to clarify that hidden nullptr semantic meaning. I am open to alternatives that clarify the three semantic meanings better.

Alex suggested the following comment for this method:

/// option value (if the option was enabled) or nil (otherwise)
const Value *value() const { return enabled() ?  &explicitOrDefaultValue : nullptr; }

and for the variable description:

/// When 'valued' is true, this is an explicitly set value.
/// Otherwise, this is the default value.
Value explicitOrDefaultValue;

In my opinion, this covers the three possible semantic meanings quite well.

I am against the suggestion in this change request because it contains arguably misleading caller instructions (where no instructions are necessary at all) and because the suggested wording implies that a caller has access to multiple option values. I do agree that "accessor" is also problematic and posted a change request with a specific replacement suggestion.

If you are adding this if statement "just in case", please do not add it. It is the caller's responsibility to check that the value exists as it is not possible to print a non-existing value correctly. This method can (and probably should) assert (using Assure()) that the value exists.

Otherwise, please discuss what caller(s) prompted you to add this if statement.

@rousskov

If you are adding this if statement "just in case", please do not add it. It is the caller's responsibility to check that the value exists as it is not possible to print a non-existing value correctly.

I assumed this method could be used for debugging purposes. In this case, as I understand it, the calling procedure might not know whether the internal variable has been set.

This method can (and probably should) assert (using Assure()) that the value exists.

Refactor it to the following, then?

template <>
inline void
TypedOption<CharacterSetOptionValue>::printValue(std::ostream &os) const
{
    Assure(recipient_->value());
    recipient_->value()->printChars(os); // TODO: Quote if needed.
}

I assumed this method could be used for debugging purposes.

I do not think that is a valid assumption in this case. Debugging code should use higher-level methods that handle non-existent values correctly.

Refactor it to the following, then?

If you really insist. FWIW, my recommendation for this method in this PR is to just add () to get this code to compile. This PR is not about improving other aspects of this method, and the proposed assertion is not related to the primary PR changes.